Usually we add the year when the file was added here (I have no idea if that's mandatory though) (applies to all new files with copyright header)

nit

I know the loggers are auto-generated, but the info messages do not add anything. Plus IIRC, controller-runtime logs all the webhook requests it receives. So that is covered.

@srm09 Afaik it doesn't log them on the default log level. But I wouldn't add log statements here. When we do it we should do it across the repo.

One more suggestion about the method, since we are refactoring it, let's. break this down into smaller chunks aimed at specific parts of the object. For example, a separate method for s.Replicas, RolloutStrategy and so on.

+1 for that suggestion

Hm usually we don't break it down that far. Not really against splitting it up, but if it was intended as more or less one func per field I think that's a bit much.

@fabriziopandini wdyt?

FYI, some of the providers to see to be breaking down the validations to smaller functions. Ref: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/master/api/v1alpha4/azuremachine_validation.go#L31-L59

Afaik we usually don't add the kind

typos 😂

I use the Intellij spellchecker. ;)

We usually seem to check if the type case works and then return an error (instead of a panic which would happen otherwise), e.g.:

        oldM, ok := old.(*Machine)
	if !ok {
		return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", old))
	}

Can you also fix it in the KubeadmControlPlane webhook please? (that one was already there before your PR :))

I added the checks in both the places now :)
I dont think the check was present before (or atleast I didnt find it 😅 ).
Anyway, it should be taken care now.

I was referring to that one: